-
Notifications
You must be signed in to change notification settings - Fork 1.9k
doc: add example for cache factory #19139
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
milenkovicm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @jizezhang I had a quick look, it does make sense.
Will have a better look later.
Hi @milenkovicm , do you have any thoughts on this PR? Thanks. |
|
apologies @jizezhang it slipped off my radar. |
milenkovicm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this makes sense @jizezhang please just consider adding some documentation
| match self { | ||
| Self::Dataframe => "dataframe", | ||
| Self::DeserializeToStruct => "deserialize_to_struct", | ||
| Self::CacheFactory => "cache_factory", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe doc should be updated cargo run --example dataframe -- [dataframe|deserialize_to_struct|cache_factory]
| .cache() | ||
| .await?; | ||
|
|
||
| println!("caching has not been executed"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this print relevant?
Hi @milenkovicm , thanks for reviewing. I added doc and a bit clean up. Please take a look again when you get a chance. Thanks! |
milenkovicm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this makes sense thanks @jizezhang
Which issue does this PR close?
CacheFactorywithDataFrame::cache#18893.Rationale for this change
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?